Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4176 customizing reminder message to partner #5093

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Sukhpreet-s
Copy link
Contributor

@Sukhpreet-s Sukhpreet-s commented Mar 12, 2025

Resolves #4176

Description

This PR adds functionality to include a customized message in the reminder email sent to partners. A rich text input field is added to the Organization edit form. It also allows interpolation for partner_name as a variable in the customized message similar to default_email_text field.

The following components are added/updated:

  • reminder_email_text rich text field is added to the Organization model.
  • The form field is added to the edit and show details Organization files.
  • The reminder_email_text is displayed in both html and text versions of the notify_deadline email.

Looking for feedback:

  1. I allowed interpolation only for partner_name variable in the customized message as none of the other values made sense to include. Is there any other value that need to be allowed here, like deadline_day?

  2. The plain text version of the email looks like below. It contains some HTML comments.

Screenshot

I've followed the DistributionMailer.partner_mailer implementation and noticed that even the DistributionMailer has the same behaviour for the plain text version of the email. I wonder if this only happens in development environment for debugging purposes... 🤔? I've opened an issue describing it in details and trying to find a fix for this.

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • Wrote test case to verify if the custom message is included in the email text when it is added by the org admin.

Also tested manually. Here are the steps:

  1. Sign in as org admin.
  2. Open org profile by clicking "My Organization" in the sidebar.
  3. Edit the profile by clicking "Edit" button at the bottom of the page.
  4. Add some text in "Reminder Email Content" input field and save.
  5. Since this email is sent via a job that runs daily at some set time, it is convenient to execute the Mailer function directly from the rails console using the following set of commands:
    • Execute rails console from terminal: bundle exec rails console
    • Get associated partner: partner = Partner.find_by_name("<Enter name of any Partner connected to your Org>")
    • Send email: ReminderDeadlineMailer.notify_deadline(partner).deliver_now
  6. The email should be sent and can be accessed from tmp/letter_opener folder in the project locally.

Screenshots

Example screenshot of the email:

The first sentence in the following screenshot is the custom reminder message.

Input field in Organization profile edit page:
Reminder email text in Organization profile details page:
Documentation update:

@cielf cielf self-requested a review March 13, 2025 16:39
Copy link
Collaborator

@cielf cielf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Sukpreet-s

Thank you for picking this up!

It works well. I'd like to make some adjustments so that it's easier for the users, though.

1/ Please move the field in Organizations to just below the reminder deadline field.

2/ Let's also change the label on it to "Additional text for reminder email." This will make it clearer that it isn't a total replacement of the reminder email text.

3/ Let's put the custom content in the email just above the 'Please contact...'

This issue was created before we wrote the user guide -- we're going to need to make some updates there as well -- are you comfortable with drafting those changes?

@Sukhpreet-s
Copy link
Contributor Author

Thank you for the review! ✨ I'll make the requested changes.

This issue was created before we wrote the user guide -- we're going to need to make some updates there as well -- are you comfortable with drafting those changes?

Sure thing 💯 I looked around the docs and it seems like the changes need to go here 💭 Is that right or am I missing some other spot in the docs?

@cielf
Copy link
Collaborator

cielf commented Mar 13, 2025

Pretty sure that's the only spot. Thanks!

@Sukhpreet-s
Copy link
Contributor Author

@cielf The changes are ready for review. The screenshots are updated in the description above for reference.

Also, I noticed the organization edit and show pages are also shown for superuser ([email protected]).

Based on the current setup, the superusers can see reminder_email_text but cannot edit it -- Is this okay? or do we want them to be able to edit it as well? 🤔

@cielf
Copy link
Collaborator

cielf commented Mar 14, 2025

As it happens, we've got a draft issue waiting regarding removing the superuser's ability to change organization settings that the org can change themselves, so I think it's ok.

cielf
cielf previously requested changes Mar 15, 2025
Copy link
Collaborator

@cielf cielf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user guide is very new and rough -- unfortunately you ran into an uncaught issue with it -- the bits that look like code in that text should be a bit more user-friendly.

Other than that I'm quite happy with this functionally -- I'm passing it on to our senior tech folk for input.

Thanks again!

Please log into Human Essentials at https://humanessentials.app before this date and submit your request if you are intending to submit an essentials request.

<br/>
<br/>
Please contact [Your bank's name] at <%= @organization.email %>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok -- I didn't realize we still had something so ... codey... in this.

Please change <%= @organization.email %> to [ Your bank's email]


<br/>
<br/>
<%= @organization.reminder_email_text %>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and then change this to [Additional text for reminder email (see below)]

@@ -76,6 +81,9 @@ If you do not pick a day, no reminder emails are sent.
#### Deadline day (Final day of the month to submit Requests)
This day will be included in the reminder email message,

#### Additional text for reminder email
If present, this text will be included in the reminder email placed at the end of the email. In the above email template, this additional message is placed in the position of `<%= @organization.reminder_email_text %>`.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If present, this text will be included in the reminder email after the default text. In the above email template, this additional message will replace [Additional text for reminder email (see below)].

@cielf cielf requested review from awwaiid and dorner March 15, 2025 22:38
@Sukhpreet-s Sukhpreet-s requested a review from cielf March 16, 2025 05:44
@cielf cielf dismissed their stale review March 16, 2025 06:47

Addressed

Copy link
Collaborator

@cielf cielf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Customizing Reminder Message to Partners
2 participants